Repository navigation
feat(node): link captured exceptions to active OpenTelemetry spans - #5289
Conversation
Generated-By: PostHog Desktop Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
Coherence2 spec problems · 0 failing chokepoints · 2 spec gaps · 0 guard failures 44 components, 5 bullets (5 invariants, 0 requirements), 2 problems Spec problems (spec --check)
Chokepoints: none declared. Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline
Close one with |
Generated-By: PostHog Desktop Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
Generated-By: PostHog Desktop Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
Node SDK compliance v2Commit: 6d25844 Capture v0 — acceptance✅ 5 passed / ❌ 0 non-passing / Drill-down: run artifacts → Harness exit: 0; report validation exit: 0. All selected cases passed and report validation succeeded. Capture v1 — acceptance✅ 5 passed / ❌ 0 non-passing / Drill-down: run artifacts → Harness exit: 0; report validation exit: 0. All selected cases passed and report validation succeeded. Capture v0 — migration✅ 55 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. Capture v1 — migration✅ 120 passed / ❌ 1 non-passing / Drill-down: run artifacts → ❌ migration/yaml-parity-v1/remote-flags-v1.feature::Omitted GeoIP settings default to enabled in flag requests — failed_assertionFlags field differs Code: Failed step: migration/yaml-parity-v1/remote-flags-v1.feature:134 the first flags request field "geoip_disable" should equal JSON false Operation: Field: Expected: false Actual value: true Harness exit: 1; report validation exit: 1. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note 🤖 Automated comment by QA Swarm — not written by a human Review: router pass and focused re-review of the repair. Verdict: ✅ APPROVE (round 2 @ cd366fa)The Webpack issue found after round 1 is fixed. A peer-free Webpack build failed before the repair and passed with the emitted helper after it. The focused trace test, package build, and edge suite pass. Key findingsNo open actionable issue. ConvergenceThe bot review and a peer-free Webpack build found the same optional import problem; cd366fa fixes it. Reviewer summaries
Previous rounds (1)round 1 @ a3b0368 — approve on code review; a later bot review found the optional import bundle problem, now fixed. Automated by QA Swarm — not a human review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3b0368c71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Generated-By: PostHog Desktop Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
|
@PostHog review --deep |
|
@pauldambra PostHog Review is reviewing this pull request. |
PostHog Review · Deep · reviewed
|
| Step | Status | Result |
|---|---|---|
| Prepare the diff | done | 4 files, 1 chunk |
| Pick perspectives | done | 5 perspectives |
| Run review passes | done | 6 issues (+1 blind-spot) |
| Merge overlapping findings | done | 7 → 4 |
| Validate findings | done | 1 kept, 3 dismissed |
| Publish | done | 1 posted (view review) |
| Resolve comments | done | 1 left for you |
Generated-By: PostHog Desktop Task-Id: a2e9675c-9ab1-4fbe-9fa3-ab616584f930
[Robot / PostHog Slack app] This draft PR was prepared by an automated agent.
Problem
An exception captured by posthog-node has no trace ID, even when the application has an active OpenTelemetry span. This makes it hard to move from the error to its trace and related logs. The contract is being defined in sdk-specs#123.
Changes
This covers all posthog-node
$exceptionevents, including the deprecated Sentry integration. It reads ambient OpenTelemetry spans, not spans made by posthog-node's own tracing API.The optional OpenTelemetry import promise is cached for the life of the process. A local Node microbenchmark measured about 18 ms for the first lookup and 0.2 microseconds for a cached lookup; this is not a production latency measurement. The feature flag and Sentry exception tests now await
flush()because the new lookup adds an async step, so fixed counts of promise turns no longer prove capture completed.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
PostHog Slack app used the existing event preparation path and the pending SDK contract. The implementation caches the optional API lookup and reads the active span after caller and request properties are merged. Local unit, edge, type, lint, format, build, and frozen lockfile checks passed. A real SDK acceptance adapter has not run the new spec scenarios.
Refs PostHog/sdk-specs#122
Created with PostHog from a Slack thread